Create a delegate for `MapboxNavigation` called `requireMapboxNavigation` by kmadsen · Pull Request #6233 · mapbox/mapbox-navigation-android

kmadsen · 2022-08-25T19:36:18Z

Description

There is some consistent boilerplate required to setup the MapboxNavigationApp. I've just spent a bit of time migrating :libnavui-androidauto: to use the MapboxNavigationApp #6141 and I've explored a few approaches with the examples repository mapbox/mapbox-navigation-android-examples#129. We also discussed some ambiguous scenarios where we want to access MapboxNavigation directly and it requires an unsafe call MapboxNavigationApp.current()!!. #6226 (comment). This delegate removes the need for !! with specific guidelines.

This is my favorite approach so far. Create a delegate that hooks up MapboxNavigation to your LifecycleOwner. I migrated a random example from the qa-test-app to show what it looks like, take a look at the FeedbackActivity. EDIT migrated another example InactiveRouteStylingActivity.

Screenshots or Gifs

Inside any LifecycleOwner you can use the requireMapboxNavigation delegate like this. And then you have access to mapboxNavigation as if you constructed it inside.

Default can be used when [MapboxNavigationApp] is setup elsewhere.

val mapboxNavigation by requireMapboxNavigation()

Initialize the [MapboxNavigationApp] when you are ready to use it

val mapboxNavigation by requireMapboxNavigation {
  MapboxNavigationApp.setup(..)
}

Initialize and setup subscriptions

val mapboxNavigation by requireMapboxNavigation(
  onInitialize = {
    MapboxNavigationApp.setup(
      NavigationOptions.Builder(this)
          .accessToken(accessToken)
          .build()
    )
  },
  onResumedObserver = object : MapboxNavigationObserver {
    override fun onAttached(mapboxNavigation: MapboxNavigation) {
      mapboxNavigation.registerLocationObserver(locationObserver)
      mapboxNavigation.registerRoutesObserver(routesObserver)
    }
    override fun onDetached(mapboxNavigation: MapboxNavigation) {
      mapboxNavigation.unregisterLocationObserver(locationObserver)
      mapboxNavigation.unregisterRoutesObserver(routesObserver)
    }
  }
)

Another example

private val mapboxNavigation by requireMapboxNavigation(
  onCreatedObserver = object : MapboxNavigationObserver {
    override fun onAttached(mapboxNavigation: MapboxNavigation) {
      binding.mapView.location.apply {
        setLocationProvider(navigationLocationProvider)
        enabled = true
      }
      mapboxNavigation.registerLocationObserver(locationObserver)
      mapboxNavigation.registerRoutesObserver(routesObserver)
    }

    override fun onDetached(mapboxNavigation: MapboxNavigation) {
      mapboxNavigation.unregisterLocationObserver(locationObserver)
      mapboxNavigation.unregisterRoutesObserver(routesObserver)
    }
   }
) {
  MapboxNavigationApp.setup(
    NavigationOptions.Builder(this)
        .accessToken(Utils.getMapboxAccessToken(this))
        .build()
  )
}

codecov · 2022-08-25T20:01:26Z

Codecov Report

Merging #6233 (8dbc73a) into main (847f8a9) will increase coverage by 0.02%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##               main    #6233      +/-   ##
============================================
+ Coverage     68.81%   68.84%   +0.02%     
- Complexity     4346     4348       +2     
============================================
  Files           649      650       +1     
  Lines         26183    26223      +40     
  Branches       3067     3075       +8     
============================================
+ Hits          18019    18052      +33     
- Misses         6998     7001       +3     
- Partials       1166     1170       +4

Impacted Files	Coverage Δ
...vigation/core/lifecycle/RequireMapboxNavigation.kt	`75.00% <75.00%> (ø)`
...x/navigation/core/lifecycle/MapboxNavigationApp.kt	`20.00% <0.00%> (+10.00%)`	⬆️

kmadsen · 2022-08-25T22:20:57Z

...est-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt

-
-    override fun onStop() {
-        super.onStop()
-        mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver)


This is a bug. If you background the app it will unsubscribe. The subscribe happens in onCreate so the subscription is lost.

...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt

...re/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegateTest.kt

qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt

libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/MapboxNavigationOwner.kt

...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt

kmadsen · 2022-09-02T03:14:58Z

...est-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt

-        mapboxNavigation.unregisterLocationObserver(locationObserver)
-        mapboxNavigation.stopTripSession()
-    }
+    private val mapboxNavigation by requireMapboxNavigation(


This never loads unless you reference it. The lazy load can be confusing.

Also, the onCreate callback from activity happens before the Lifecycle onCreate. That causes a crash if you try to reference this delegate without using Lifecycle callbacks.

Fixed it but was finding another race condition along the way.

mapboxNavigation.setRoutes(listOf(route)) binding.startNavigation.visibility = View.GONE startSimulation(mapboxNavigation.getRoutes()[0]) <=== crashes because routes are not set yet

...avigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt

…ion` (#6233) * Create a delegate for MapboxNavigation * Make requireMapboxNavigation attach during initialization

kmadsen requested a review from a team as a code owner August 25, 2022 19:36

kmadsen mentioned this pull request Aug 25, 2022

[Android Auto] Remove MapboxNavigation from RoutePreviewCarContext #6226

Merged

kmadsen force-pushed the km-create-navigation-required-delegate branch from 7d11345 to 0f6a748 Compare August 25, 2022 22:10

kmadsen commented Aug 25, 2022

View reviewed changes

kmadsen force-pushed the km-create-navigation-required-delegate branch 2 times, most recently from e92feb7 to e8c6b00 Compare August 26, 2022 00:25

dzinad reviewed Aug 26, 2022

View reviewed changes

...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt Show resolved Hide resolved

...re/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegateTest.kt Show resolved Hide resolved

Zayankovsky reviewed Aug 26, 2022

View reviewed changes

qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt Outdated Show resolved Hide resolved

kmadsen force-pushed the km-create-navigation-required-delegate branch 4 times, most recently from 20250d4 to 254b936 Compare August 30, 2022 17:07

kmadsen commented Aug 30, 2022

View reviewed changes

libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/MapboxNavigationOwner.kt Show resolved Hide resolved

kmadsen mentioned this pull request Aug 30, 2022

Fix MapboxNavigation#detach issue #6245

Merged

kmadsen force-pushed the km-create-navigation-required-delegate branch from 027af78 to 1d63ae9 Compare August 31, 2022 16:45

kmadsen commented Sep 1, 2022

View reviewed changes

...n-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationDelegate.kt Show resolved Hide resolved

kmadsen force-pushed the km-create-navigation-required-delegate branch 3 times, most recently from 64e6ff6 to 13785bf Compare September 1, 2022 19:41

kmadsen mentioned this pull request Sep 2, 2022

androidauto emulator fails to start #5880

Closed

kmadsen commented Sep 2, 2022

View reviewed changes

Zayankovsky approved these changes Sep 5, 2022

View reviewed changes

Zayankovsky reviewed Sep 5, 2022

View reviewed changes

...avigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt Outdated Show resolved Hide resolved

kmadsen force-pushed the km-create-navigation-required-delegate branch from 8a8286f to aa87a7d Compare September 6, 2022 16:57

kmadsen commented Sep 6, 2022

View reviewed changes

...avigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt Show resolved Hide resolved

kmadsen force-pushed the km-create-navigation-required-delegate branch from aa87a7d to 6234305 Compare September 6, 2022 18:22

Create a delegate for MapboxNavigation

b145c53

kmadsen force-pushed the km-create-navigation-required-delegate branch from 6234305 to b145c53 Compare September 6, 2022 18:23

Make requireMapboxNavigation attach during initialization

8dbc73a

kmadsen force-pushed the km-create-navigation-required-delegate branch from 0b3b832 to 8dbc73a Compare September 6, 2022 18:53

Zayankovsky approved these changes Sep 6, 2022

View reviewed changes

kmadsen merged commit e3e78f4 into main Sep 6, 2022

kmadsen deleted the km-create-navigation-required-delegate branch September 6, 2022 20:06

kmadsen mentioned this pull request Sep 8, 2022

Make MapboxNavigationApp.setup create new instance #6288

Merged

Guardiola31337 pushed a commit that referenced this pull request Dec 22, 2022

Create a delegate for MapboxNavigation called `requireMapboxNavigat…

273f059

…ion` (#6233) * Create a delegate for MapboxNavigation * Make requireMapboxNavigation attach during initialization

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a delegate for `MapboxNavigation` called `requireMapboxNavigation`#6233

Create a delegate for `MapboxNavigation` called `requireMapboxNavigation`#6233
kmadsen merged 2 commits intomainfrom
km-create-navigation-required-delegate

kmadsen commented Aug 25, 2022 •

edited

Loading

Uh oh!

codecov bot commented Aug 25, 2022 •

edited

Loading

Uh oh!

kmadsen Aug 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kmadsen Sep 2, 2022

Uh oh!

kmadsen Sep 2, 2022

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

kmadsen commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Screenshots or Gifs

Default can be used when [MapboxNavigationApp] is setup elsewhere.

Initialize the [MapboxNavigationApp] when you are ready to use it

Initialize and setup subscriptions

Another example

Uh oh!

codecov bot commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kmadsen Aug 25, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kmadsen Sep 2, 2022

Choose a reason for hiding this comment

Uh oh!

kmadsen Sep 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kmadsen commented Aug 25, 2022 •

edited

Loading

codecov bot commented Aug 25, 2022 •

edited

Loading